The second commit fixes a subtle issue in the master branch, where SameBoy requests to fread the rounded-up number of bytes from the rom file. This silently succeeds because the return value of fread is not checked.

I blindly copied the same pattern with my zip patch, but later realized the issue. I am not sure what the behavior of minzip is in this case (it might overwrite the extra space w/ junk data?)... I didn't bother to even check, and opted to just make the code do the "right thing" instead.

Few notes:

• I think it would be better to have this code as part of the SDL frontend rather than the core itself. The current changeset also breaks the non-SDL executable targets (Cocoa, iOS, tester...) because they're not linked against minizip
• Use GB_HAS_MINIZIP rather than HAVE_MINIZIP to not pollute the global "namespace"
• Might want to automatically set it to 0 if minizip is not found
• Last one's a bit tricky; on Windows, minizip's DLL is expected to be copied to bin/SDL folder (similarly to SDL2.dll). Additionally, the Windows release will redistribute that DLL, minizip's license needs to be appended to the bin/SDL/LICENSE file.

Sounds good, I will work on that.

I don't understand your last bullet point, though. (I haven't hset up a GnuWin32 build environment yet, so I'm not familiar with how everything is laid out there.) Are you green-lighting my suggestion to bring the latest minizip into the tree, rather than link against it as an external dependency?

Having a copy of minizip in the tree would sure simplify things (just make sure to update .gitattributes and mark it as linguist-vendored), so only the license issue remains for that bullet.

Made some minor changes as you suggested, but I did not refactor the zipped rom loading code into SDL/.

While I was working on this I realized a few things:

• As it stands, Cocoa and iOS targets should be able to just link against minizip (whether that ends up in-tree or out) and it should just work (although I'm unable to test this). If i refactor zipped rom loading out of Core/, it could go into SDL/, as you suggested, but then iOS and Cocoa would be missing the feature. This could be solved by adding a new layer/module (could call it "Common/" or similar), but someone else would have to add support for that to the iOS/Cocoa code, since I don't have a mac. So you have three choices: A) accept this as-is; B) have me move zipped rom loading into SDL/, dropping support for Apple devices, or, C) I can create a new "Common/" layer in-between SDL/ and Core/, and then you can have someone who owns a Mac add support for Common/ to the iOS and Cocoa targets.

(Sorry if this was already obvious when you asked me to move it to SDL, I'm new here ;)

• The Windows build environment, GnuWin32, looks like it was last updated in 2010; and it's version of zlib is from 2005 and doesn't contain minizip. Is there any interest in migrating to MSYS2/MinGW-w64/Cygwin? In that case, we could keep minizip out of tree, which would partially resolve the licensing issue. (I tried putting minizip in tree locally, and it didn't simplify the makefile at all, as it still depends on external zlib, which you have to test for anyway.)

I have a feeling it will be really easy to get SameBoy working under MSYS2 or one of the others, in fact I wouldn't be surprised if it compiles cleanly as-is, in which case we just need to update the build faq and nothing else.

fwiw, my vote is to create a "Common/" module, migrate to MSYS2/MinGW, and keep minizip out of tree.

I guess we still have to figure out the licensing thing in order to distribute binaries to Windows users... groan

Windows has basic native zip support (enum and extract), just use IShellFolder. Should work as far back as XP, maybe even 2000.

Not exactly this exact topic but close enough to put here but.

While Zip is a common file, 7-zip seems to be increasingly used due to its higher compression ratio. Wouldn't it be a good idea to include this format with supporting other compressed archives?

Are there any more problems that exist?